This way of adding search text for custom components may need to be refactored at some point (though not in this pull request :) ).

One option could be to create a custom <SearchableText>...</SearchableText> component that sends search text to the settings screen with a custom React context and provider.

For now let's put only emailToNoteLabel() and emailToNoteDescription() in there

🚧 Possible crash 🚧: Be careful here! Currently, accountTypeToString throws when the account type is unknown. If we eventually add a new account type, this might cause the app to crash if users with this account type open the JoplinCloud config screen on an old client.

Consider including a check that accountType actually is a valid account type, and displaying 'Unknown' if not.

I added a function that tries to get the value, if it fails it logs the error message and returns 'Unknown', I guess that works, right?

We should probably default to using useCallback for event handlers, especially when it's dependent on a prop like here. Probably the prop won't change but if for some reason it's initially empty, then set to a value, the component will incorrectly use the empty value

but if for some reason it's initially empty, then set to a value, the component will incorrectly use the empty value

I'm not sure that's true... I think useCallback prevents unnecessary re-renders of child components (rather than forcing the function to reload). More specifically, suppose that props.joplinCloudWebsite changes from "" to "https://joplincloud.com/":

1. props.joplinCloudWebsite is initially "". This is the initial render.
   • goToJoplinCloudProfile is set to a function. props.joplinCloudWebsite is "" in its scope. As such, if goToJoplinCloudProfile is called, it will openExternal('/users/me').
   • The component renders. A Button is given goToJoplinCloudProfile as a prop. This is the first render of that Button.
2. props.joplinCloudWebsite changes to "https://joplincloud.com/". This causes the component to re-render.
   • goToJoplinCloudProfile is set to a function. props.joplinCloudWebsite is "https://joplincloud.com/" in its scope. As such, if goToJoplinCloudProfile is called, it will openExternal('https://joplincloud.com//users/me').
   • The component renders. A Button is given goToJoplinCloudProfile as a prop. This prop is different from its previous value (because function(){} !== function(){}). As a result, the Button re-renders.
     • This re-render means that the button will use the new version of goToJoplinCloudProfile.

Observe that in step 2, because React considers different functions to be unequal, the Button uses the new version of the function that includes "https://joplincloud.com/" for props.joplinCloudWebsite in its scope.

Also see react.dev's "Should you add useCallback everywhere".

Ah yes, that's true, it will re-render anyway.

But I actually think that it's reasonable to default to adding useCallback rather than wondering whether it needs to be optimised or not. In this case maybe it doesn't make a difference that it re-render even when the prop doesn't change, but maybe it does, who knows. That's the idea of mostly defaulting to using useCallback - then we don't need to wonder about this, and it feels like it's more future proof

Is it possible to avoid these CSS calculations? As I think that will make it difficult to maintain a consistent style throughout the application. We have the --joplin-margin - could that be enough? Or if something else is needed maybe introduce a new margin?

This AccountType could be used elsewhere, including in joplinCloud.ts so I think this file should be named more generically. Maybe just types.ts.

Even accountTypeToString is type-related since it converts from one type to another